Skip to content

feat(api): add scan security verification endpoint and non-suspicious filters#537

Closed
VACInc wants to merge 6 commits intoopenclaw:mainfrom
VACInc:feat/v1-scan-verification-nonsuspicious
Closed

feat(api): add scan security verification endpoint and non-suspicious filters#537
VACInc wants to merge 6 commits intoopenclaw:mainfrom
VACInc:feat/v1-scan-verification-nonsuspicious

Conversation

@VACInc
Copy link
Contributor

@VACInc VACInc commented Feb 26, 2026

Summary

Adds API support for skill security scan verification and aligns API filtering with website behavior for hiding suspicious skills.

What Changed

  • Added GET /api/v1/skills/{slug}/scan for normalized security scan verification details.
  • Added merged VT + LLM security snapshot output for version and scan responses.
  • Added nonSuspiciousOnly support to:
    • GET /api/v1/search
    • GET /api/v1/skills
  • Added legacy /api/search boolean alias parity (true and 1).
  • Refactored boolean query parsing into shared convex/lib/httpUtils.ts to remove duplication.
  • Fixed trending list behavior for nonSuspiciousOnly=true to backfill clean entries before enforcing the final limit.
  • Added/updated regression tests for shared boolean parsing and trending non-suspicious backfill behavior.
  • Updated API docs and OpenAPI for the new endpoint, fields, and query params.

Validation

  • bun run lint
  • bun run test
  • bun run test:e2e

Files

  • convex/httpApiV1/skillsV1.ts
  • convex/skills.ts
  • convex/httpApi.ts
  • convex/lib/httpUtils.ts
  • convex/lib/httpUtils.test.ts
  • convex/httpApiV1.handlers.test.ts
  • convex/httpApi.handlers.test.ts
  • convex/skills.listPublicPage.test.ts
  • docs/http-api.md
  • docs/api.md
  • public/api/v1/openapi.json

Relates #189

@vercel
Copy link
Contributor

vercel bot commented Feb 26, 2026

Someone is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR adds a GET /api/v1/skills/{slug}/scan endpoint that returns a merged VT + LLM security snapshot, adds nonSuspiciousOnly filtering to the skills list and search endpoints (with legacy nonSuspicious alias parity), and refactors boolean query-param parsing into a shared parseBooleanQueryParam utility.

The implementation is well-structured and the two issues flagged in previous review threads have been addressed:

  • normalizeSecurityStatus now explicitly maps 'completed''error' rather than falling through to 'pending'
  • The /scan response now exposes moderation.matchesRequestedVersion and moderation.sourceVersion to make the version-skew between the skill-level moderation snapshot and the per-version scan data fully transparent to consumers

Key highlights:

  • buildSkillSecuritySnapshot correctly merges VT and LLM statuses by priority (malicious > suspicious > pending > error > clean), and hasScanResult is correctly true only when a definitive verdict exists from at least one scanner
  • The trending-list backfill (fetching up to MAX_PUBLIC_LIST_LIMIT before enforcing limit) handles nonSuspiciousOnly filtering without under-filling the page
  • Boolean parsing is consistent across both the v1 and legacy /api/search handlers
  • Test coverage is thorough and explicitly validates the edge cases (completed-without-verdict, VT-only, LLM-only, historical-version moderation context)

Confidence Score: 4/5

  • Safe to merge — logic is correct, edge cases are tested, and prior review concerns are resolved.
  • The security snapshot merging logic, endpoint routing, and boolean-param refactor are all implemented correctly and covered by targeted regression tests. One point withheld: for cursor-based pagination (non-trending sorts like downloads, stars, etc.), the nonSuspiciousOnly filter is applied post-paginate, which can silently return fewer items than limit while still yielding a non-null nextCursor. This is a known pagination-with-post-filter trade-off and the trending path (the more prominent case) is correctly handled via backfill, but it may surprise API consumers on other sort modes if not surfaced in documentation.
  • No files require special attention.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: convex/skills.ts
Line: 2276-2282

Comment:
**Post-pagination filtering may silently under-fill pages for cursor-based sorts**

For the `updated` sort (and the non-trending paginate path below), `filterPublicSkillPage` is applied *after* `paginate`, so when `nonSuspiciousOnly=true` removes suspicious skills the response can return fewer items than `limit` while still setting a non-null `nextCursor`. For example, if 4 of the 10 requested items are suspicious, the consumer gets 6 items but a valid cursor — then gets another partially-filled page on the next request.

The `trending` path correctly solves this by fetching up to `MAX_PUBLIC_LIST_LIMIT` and only stopping at `limit` clean entries. A similar over-fetch-and-trim strategy could be applied to cursor-based sorts, but at minimum the public docs and OpenAPI descriptions for `nonSuspiciousOnly` should note that page sizes may be smaller than `limit` when the filter is active — otherwise callers may incorrectly interpret a short page as end-of-results.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: 387160d

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b46cb2938e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44e694c974

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ngutman
Copy link
Contributor

ngutman commented Mar 12, 2026

Reviewed this PR. The feature itself still looks useful and needed: main already supports nonSuspiciousOnly in the underlying search/list code, but the public HTTP API still doesn’t expose it, and /api/v1/skills/{slug}/scan doesn’t exist yet.

That said, I don’t think this branch can be reviewed further as-is. It’s stale against current main, and the conflicts are not just mechanical. A straight conflict resolution that favors this branch would regress newer API behavior that landed after this PR was opened.

Required updates before further review:

  • Rebase/cherry-pick onto current main and resolve conflicts carefully.
  • Preserve the current /api/v1/skills/:slug/moderation route and its evidence-redaction behavior.
  • Preserve the current v1 response shape additions on main (latestVersion.license, moderation verdict, reasonCodes, summary, engineVersion, updatedAt).
  • Preserve the current publish safeguards (acceptLicenseTerms checks) and the existing transfer routes under /api/v1/skills/:slug/transfer.

So my take is: keep the intent of this PR, but update it on top of current main and then I can take another pass.

@VACInc VACInc force-pushed the feat/v1-scan-verification-nonsuspicious branch from e1c0fdb to de430fa Compare March 12, 2026 11:04
@VACInc
Copy link
Contributor Author

VACInc commented Mar 12, 2026

@greptile-apps review
@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. What shall we delve into next?

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@VACInc
Copy link
Contributor Author

VACInc commented Mar 12, 2026

@greptile-apps review
@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e7ce157c3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@VACInc
Copy link
Contributor Author

VACInc commented Mar 12, 2026

@greptile-apps review
@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@VACInc
Copy link
Contributor Author

VACInc commented Mar 12, 2026

@ngutman updated; would you mind taking another peek when you get a moment?

I had originally meant to handle the CLI as well but I am not sure how you feel about that. Maybe we get this one in and I'll open a new one for the CLI once this one has merged?

Let me know your thoughts

@ngutman
Copy link
Contributor

ngutman commented Mar 13, 2026

Closing this in favor of #820 because that replacement PR carries the reviewed fixes on a branch we can update directly.

Thanks for the original work here and for the solid starting point.

@ngutman ngutman closed this Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants